Skip to content

Conversation

RalfJung
Copy link
Member

@Urgau noted in #140792 that fallback bodies our backends don't use are untested... which is correct, and it is a problem. So this adds a testing-only flag to Miri to force the use of fallback bodies, and adds a run of the Miri test suite with that flag to CI. This should not take much more than a minute so I hope it's fine? Let's see how long it actually takes.

While at it, I made that test run also enable MIR optimizations. Miri's CI has a run with that, and it has caught mir-opt bugs in the past -- this way we'd see the CI failure earlier.

r? @scottmcm

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels May 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 10, 2025

The Miri subtree was changed

cc @rust-lang/miri

@WaffleLapkin
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 11, 2025

📌 Commit 6814c38 has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request May 11, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#140792 (Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations)
 - rust-lang#140795 (Prefer to suggest stable candidates rather than unstable ones)
 - rust-lang#140865 (Make t letter looks like lowercase rather than uppercase)
 - rust-lang#140878 (Two expand-related cleanups)
 - rust-lang#140882 (Split duration_constructors to get non-controversial constructors out)
 - rust-lang#140886 (Update deps of bootstrap for Cygwin)
 - rust-lang#140903 (test intrinsic fallback bodies with Miri)

r? `@ghost`
`@rustbot` modify labels: rollup
@RalfJung
Copy link
Member Author

Something seems to have gone wrong, it doesn't look like it ran any tests in the CI run.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 11, 2025
@bors bors merged commit 615b447 into rust-lang:master May 11, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 11, 2025
Rollup merge of rust-lang#140903 - RalfJung:fallback-body-tests, r=WaffleLapkin

test intrinsic fallback bodies with Miri

`@Urgau` noted in rust-lang#140792 that fallback bodies our backends don't use are untested... which is correct, and it is a problem. So this adds a testing-only flag to Miri to force the use of fallback bodies, and adds a run of the Miri test suite with that flag to CI. This should not take much more than a minute so I hope it's fine? Let's see how long it actually takes.

While at it, I made that test run also enable MIR optimizations. Miri's CI has a run with that, and it has caught mir-opt bugs in the past -- this way we'd see the CI failure earlier.

r? `@scottmcm`
@rustbot rustbot added this to the 1.89.0 milestone May 11, 2025
@RalfJung
Copy link
Member Author

Damn, the rollup landed it.^^ Will post a follw-up...

@RalfJung RalfJung deleted the fallback-body-tests branch May 11, 2025 07:28
@scottmcm
Copy link
Member

Thanks for working on this! I'd be quite happy to remove the one-off testing for some of them in

reason = "The fallbacks will never be stable, as they exist only to be called \
by the fallback MIR, but they're exported so they can be tested on \
platforms where the fallback MIR isn't actually used",

that I'd added, but not done consistently.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 17, 2025
checktools.sh: fix bashism

Follow-up to rust-lang#140903. Turns out `tests/{pass,panic}` only properly expands in bash, not in dash. :/

r? `@WaffleLapkin`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 17, 2025
Rollup merge of rust-lang#140917 - RalfJung:checktools, r=WaffleLapkin

checktools.sh: fix bashism

Follow-up to rust-lang#140903. Turns out `tests/{pass,panic}` only properly expands in bash, not in dash. :/

r? `@WaffleLapkin`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request May 18, 2025
checktools.sh: fix bashism

Follow-up to rust-lang/rust#140903. Turns out `tests/{pass,panic}` only properly expands in bash, not in dash. :/

r? `@WaffleLapkin`
@RalfJung
Copy link
Member Author

@scottmcm yeah those can probably be removed now, just make sure the relevant intrinsic has coverage in Miri. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants